Conversation
google-cla
bot
added
the
cla: yes
product-auto-label
bot
added
the
api: spanner
c24t
force-pushed
the
depend-sqlparse
branch
from
3db4e5f to
ff90523
Compare
mf2199
left a comment
mf2199
left a comment
There was a problem hiding this comment.
👍 Good catch! This is something I wasn't sure about adding on a global scale.
| "proto-plus==1.11.0", | ||
| "libcst >= 0.2.5", | ||
| "proto-plus == 1.11.0", | ||
| "sqlparse >= 0.3.0", |
There was a problem hiding this comment.
sqlparse is only used for dbapi and is not required for using the Cloud Spanner client. I think it makes more sense to have this as a separate extra dependency similar to the OpenTelemetry dependencies under "tracing".
There was a problem hiding this comment.
The problem with making this an optional dependency is that we've got code in the library that will fail to import without it. The library still works without tracing, it just doesn't emit traces.
If we want to avoid sqlparse as a required dependency I see two options: (1) make it an optional import and catch the import errors in the DBAPI package and log an error, or (2) make spanner_dbapi a separate installable package in this repo, with its own set of requirements.
There was a problem hiding this comment.
As discussed I think (2) is the best option
larkee
left a comment
larkee
left a comment
There was a problem hiding this comment.
LGTM. I would prefer to have spanner_dbapi as a separate installable before doing a release but I won't block you on this.
| "proto-plus==1.11.0", | ||
| "libcst >= 0.2.5", | ||
| "proto-plus == 1.11.0", | ||
| "sqlparse >= 0.3.0", |
There was a problem hiding this comment.
As discussed I think (2) is the best option
🤖 I have created a release \*beep\* \*boop\* --- ## [2.1.0](https://github.com/googleapis/python-spanner/compare/v2.0.0...v2.1.0) (2020-11-24) ### Features * **dbapi:** add aborted transactions retry support ([#168](https://github.com/googleapis/python-spanner/issues/168)) ([d59d502](https://github.com/googleapis/python-spanner/commit/d59d502590f618c8b13920ae05ab11add78315b5)), closes [#34](https://github.com/googleapis/python-spanner/issues/34) [googleapis/python-spanner-django#544](https://github.com/googleapis/python-spanner-django/issues/544) * remove adding a dummy WHERE clause into UPDATE and DELETE statements ([#169](https://github.com/googleapis/python-spanner/issues/169)) ([7f4d478](https://github.com/googleapis/python-spanner/commit/7f4d478fd9812c965cdb185c52aa9a8c9e599bed)) ### Bug Fixes * Add sqlparse dependency ([#171](https://github.com/googleapis/python-spanner/issues/171)) ([e801a2e](https://github.com/googleapis/python-spanner/commit/e801a2e014fcff66a69cb9da83abedb218cda2ab)) ### Reverts * Revert "test: unskip list_backup_operations sample test (#170)" (#174) ([6053f4a](https://github.com/googleapis/python-spanner/commit/6053f4ab0fc647a9cfc181e16c246141483c2397)), closes [#170](https://github.com/googleapis/python-spanner/issues/170) [#174](https://github.com/googleapis/python-spanner/issues/174) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
3 participants
#160 added
sqlparseas a test dependency only, but included non-test code that imports it. Running the tests outside of nox gives this error:E ModuleNotFoundError: No module named 'sqlparse'.This PR makes
sqlparsea regular non-test dependency.The version number comes from googleapis/python-spanner-django@2b096c5.